Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle OS Interruptions #217

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Handle OS Interruptions #217

merged 1 commit into from
Jan 25, 2024

Conversation

jsturtevant
Copy link
Collaborator

When running on MacOs we found a bug:

Dec 12 18:45:42 spin-oci-worker containerd[102]: time="2023-12-12T18:45:42.523982005Z" level=info msg="Shim successfully started, waiting for exit signal..."
Dec 12 18:45:42 spin-oci-worker containerd[102]: time="2023-12-12T18:45:42.533327005Z" level=error msg="listener accept got Os { code: 4, kind: Interrupted, message: "Interrupted system call" }"
Dec 12 18:45:42 spin-oci-worker containerd[102]: time="2023-12-12T18:45:42.535132005Z" level=info msg="ttrpc server listener stopped"

This will retry the connection. I don't have a Mac to test this on but wanted to open it up for feedback incase others are seeing the error

@jsturtevant
Handle OS interruptions
693f4c5 
It is possible when the socket is being read we can receive an OS Interrupt. As this isn't a hard error, in this case retry instead of shutting down the ttrpc server

Signed-off-by: James Sturtevant <[email protected]>
@codecov Codecov
Copy link

codecov bot commented Dec 12, 2023 •
edited
Loading

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (6520f9e) 24.35% compared to head (693f4c5) 24.33%.
Report is 4 commits behind head on master.

Files Patch % Lines
src/sync/server.rs 0.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #217      +/-   ##
==========================================
- Coverage   24.35%   24.33%   -0.03%     
==========================================
  Files          16       16              
  Lines        2652     2655       +3     
==========================================
  Hits          646      646              
- Misses       2006     2009       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Tim-Zhang
Tim-Zhang approved these changes Jan 3, 2024
View reviewed changes
Copy link
Member

@Tim-Zhang Tim-Zhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @jsturtevant

@Tim-Zhang Tim-Zhang requested review from lifupan and teawater January 3, 2024 13:03
@devigned
Copy link

devigned commented Jan 3, 2024

I'm going to test this out on my OSX machine and report back.

@devigned
Copy link

devigned commented Jan 4, 2024

It took rebuilding the tree of dependencies, but it seems to be working on my OSX arm64 machine.

@jsturtevant
Copy link
Collaborator Author

@devigned Thanks for testing it out!

@jsturtevant
Copy link
Collaborator Author

@Tim-Zhang @wllenyj can we merge this?

@wllenyj wllenyj merged commit 6f12d9c into containerd:master Jan 25, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wllenyj wllenyj wllenyj approved these changes

@Tim-Zhang Tim-Zhang Tim-Zhang approved these changes

@lifupan lifupan Awaiting requested review from lifupan

@teawater teawater Awaiting requested review from teawater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsturtevant @devigned @Tim-Zhang @wllenyj